Repository navigation
Add a real WebSocket builtin module - #510
Open
dan-distributive wants to merge 2 commits into
Open
dan-distributive wants to merge 2 commits into
dan-distributive wants to merge 2 commits into
Conversation
pythonmonkey has never had a WebSocket implementation. dcp-client's own SocketIOTransport.buildOptions() explicitly special-cases this: every platform except pythonmonkey gets transports: ['websocket', 'polling'] and upgrades to a persistent WebSocket within the first round trip or two; pythonmonkey is hard-restricted to ['polling'] forever, since `typeof WebSocket` was previously `undefined`. That forces pythonmonkey to be the only client that ever *sustains* HTTP long-polling for an entire connection's lifetime against services like DCP's package manager - a code path essentially unexercised by any other real client, where a real, reproducible session-routing bug lives (a freshly issued session id doesn't reliably route to the same backend on the very next polling request; confirmed independent of pythonmonkey/dcp-client with plain curl, aiohttp, and Node https). Rather than work around that bug, this gives pythonmonkey a real WebSocket implementation - a JS class matching the DOM API (builtin_modules/WebSocket.js), backed by a Python aiohttp-based implementation (builtin_modules/WebSocket-internal.py), following the exact existing pattern XMLHttpRequest already uses (loaded dynamically via require() at pythonmonkey import time, no C++ rebuild needed). Two real bugs were found and fixed while building it (documented inline in WebSocket.js/WebSocket-internal.py's own docstrings): - A race between the 'open' event firing and send/close functions being available, since real clients often send their first message immediately on 'open'. - engine.io-client's real WS transport unconditionally calling `this.ws._socket.unref()` (a Node `ws`-library-shaped assumption that doesn't hold for a browser-style WebSocket), worked around with a harmless no-op `_socket` property. This alone doesn't change any existing behavior - dcp-client's platform check still needs its pythonmonkey exclusion relaxed on its own side for anything to actually use this. Verified via a real end-to-end DCP job (job.exec(), real network dispatch to the demo/dcp compute group) after also relaxing that check locally. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
- A refused connection now fires 'close' (1006) after 'error' and ends in readyState CLOSED instead of staying CONNECTING forever. - close() while still connecting is honoured once the socket opens, instead of being ignored and 'open' firing anyway. - Surface the negotiated subprotocol and the server's close reason. - wasClean is false only for 1006 (no close handshake), not for any non-1000 code. - Send errors are reported via 'error' instead of unhandled rejections; send() accepts any ArrayBufferView and rejects other types. - Drop the unused pre-open send buffer and headers parameter; trim file headers and comments to the why; add WebSocket-internal.d.ts. - Add tests/python/test_websocket.py covering the above against a local aiohttp server. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Adds a
WebSocketbuiltin module: a JS class matching the DOM API (builtin_modules/WebSocket.js) backed by aiohttp (builtin_modules/WebSocket-internal.py), following the same pattern asXMLHttpRequest.WebSocket,MessageEventandCloseEventare exposed on the global object.Motivation: pythonmonkey has been the only dcp-client platform restricted to Socket.IO long-polling, because it had no
WebSocket. That sustained-polling path is exercised by no other client and has a reproducible session-routing bug in the package-manager service. A realWebSocketlets pythonmonkey take the same upgrade path every other platform takes. dcp-client's platform check still needs relaxing on its own side before anything uses this, so this PR changes no existing behaviour.Two consumer-facing details worth knowing:
openevent, so a message sent from anopenhandler (as engine.io does) can't race them._socket.unref()is provided because engine.io-client calls it on open whenautoUnrefis set, a Nodews-library detail.Test plan
tests/python/test_websocket.py(new): text and binary round trips, subprotocol negotiation, server-initiated close with code and reason, refused connection (error then close 1006, readyState CLOSED), andclose()while still connecting. Runs against a local aiohttp server.job.exec()to thedemocompute group with dcp-client's platform check relaxed locally.The one red CI job on the first push was the docs step's doxygen 1.9.7 download returning 404, which #509 fixes; it is unrelated to this change.
Note on authorship
Largely produced by Claude (Anthropic's Claude Code) under my direction, then audited and exercised against a live server.
🤖 Generated with Claude Code